workflow which ensures that talawa Api app starts in docker #2759
Conversation
WalkthroughThis pull request introduces a new Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
366-372: Use environment variable for health check URLThe health check URL is hardcoded. Consider using the
HEALTH_CHECK_URLenvironment variable:-until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://talawa-api-dev:4000/health" 2>&1 || [ $timeout -eq 0 ]; do +until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "${HEALTH_CHECK_URL}/health" 2>&1 || [ $timeout -eq 0 ]; do🧰 Tools
🪛 actionlint (1.7.4)
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 366-366: trailing spaces
(trailing-spaces)
392-407: Fix environment variables indentationThe
env:section should be at the same level assteps::- env: - HEALTH_CHECK_URL: http://localhost:4000 - COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }} + env: + HEALTH_CHECK_URL: http://localhost:4000 + COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 407-407: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 407-407: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
|
making changes as suggested |
…ansh-talawa-api into api_fail_docker merge# Please enter a commit message to explain why this merge is necessary,
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
311-320: Optimize cache configurationConsider adding package-lock.json to the cache key for better cache hits.
- name: Cache Node.js dependencies uses: actions/cache@v4 with: path: | ~/.npm node_modules - key: ${{ runner.os }}-docker-check-${{ hashFiles('**/package-lock.json') }} + key: ${{ runner.os }}-docker-check-node-${{ hashFiles('package-lock.json') }} restore-keys: | - ${{ runner.os }}-docker-check- + ${{ runner.os }}-docker-check-node-
333-337: Improve error handling in container startupConsider capturing and displaying the exit code for better debugging.
-if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then +if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then + exit_code=$? echo "Failed to start containers" + echo "Exit code: $exit_code" docker-compose -f docker-compose.dev.yaml logs exit 1 fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 407-407: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
307-307: Fix action step configuration syntax
The Node.js setup step has incorrect syntax.
-- uses: actions/setup-node@v4
+ - uses: actions/setup-node@v4392-407: Verify environment variable configuration
Please verify that these environment variables match the ones in docker-compose.dev.yaml.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 407-407: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
393-407: LGTM: Environment variables are well configuredThe environment variables are properly set with appropriate use of secrets. Just remove trailing spaces at line 407.
- MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} - + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}
299-407: Consider enhancing the Docker health check implementationSome suggestions to improve the implementation:
- Consider using Docker's built-in HEALTHCHECK instruction in Dockerfile instead of shell-based health checks
- Consider implementing exponential backoff for health checks instead of fixed intervals
- Consider adding container resource limits to prevent resource exhaustion
These changes would make the health checks more robust and maintainable.
🧰 Tools
🪛 actionlint (1.7.4)
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 408-408: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
299-320: LGTM: Job configuration follows best practices
The job configuration is well-structured with proper dependency management and caching strategy.
353-364:
Fix Redis health check syntax
There's a syntax error in the if condition:
- if[ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; thenLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
321-337: Fix formatting and enhance error logging.
- Remove trailing spaces after
run :and in the if condition- Add container logs to the error message when docker-compose file is not found
- run : | + run: | # Ensure no containers are running docker-compose -f docker-compose.dev.yaml down -v || true # Verify docker-compose file exists - if [ ! -f "docker-compose.dev.yaml" ]; then + if [ ! -f "docker-compose.dev.yaml" ]; then echo "Error: docker-compose.dev.yaml not found" + docker-compose -f docker-compose.dev.yaml logs exit 1 fi🧰 Tools
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
366-382: Enhance API health check implementation.Consider adding additional health check parameters to ensure comprehensive API readiness:
- Add request timeout to curl command
- Check for specific health check response content
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do + until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf --max-time 5 "http://localhost:4000/health" 2>&1 | grep -q '"status":"healthy"' || [ $timeout -eq 0 ]; do🧰 Tools
🪛 yamllint (1.35.1)
[error] 366-366: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
384-384: could not parse as YAML: yaml: line 384: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 409-409: trailing spaces
(trailing-spaces)
[error] 388-388: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
299-320: LGTM! Job configuration follows best practices.
The job configuration is well-structured with proper dependency setup, Node.js configuration, and dependency caching.
299-409: Overall implementation achieves the PR objective.
The Docker check implementation successfully verifies that the Talawa API starts correctly in a Docker environment. It includes proper health checks, timeouts, and cleanup procedures. The remaining issues are mostly syntax and formatting related, which have been addressed in previous comments.
🧰 Tools
🪛 actionlint (1.7.4)
384-384: could not parse as YAML: yaml: line 384: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 409-409: trailing spaces
(trailing-spaces)
[error] 388-388: syntax error: could not find expected ':'
(syntax)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
367-380: Fix indentation in API health check.The indentation is inconsistent in this section, which affects readability.
- # Wait for TALAWA API to be healthy - until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do - echo "Waiting for API to start... ($timeout seconds remaining)" - sleep 1 - ((timeout--)) - done + # Wait for TALAWA API to be healthy + until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do + echo "Waiting for API to start... ($timeout seconds remaining)" + sleep 1 + ((timeout--)) + done🧰 Tools
🪛 yamllint (1.35.1)
[error] 367-367: trailing spaces
(trailing-spaces)
394-409: Fix trailing spaces in environment variables section.Remove trailing spaces to maintain consistent formatting.
- MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} - + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} +🧰 Tools
🪛 yamllint (1.35.1)
[warning] 395-395: wrong indentation: expected 6 but found 8
(indentation)
[error] 409-409: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 367-367: trailing spaces
(trailing-spaces)
[warning] 395-395: wrong indentation: expected 6 but found 8
(indentation)
[error] 409-409: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)
299-320: LGTM! Job configuration follows best practices.
The job configuration is well-structured with proper dependency management, Node.js setup, and dependency caching.
321-338: LGTM! Docker startup implementation is robust.
The implementation includes proper error handling, timeouts, and cleanup of existing containers.
🧰 Tools
🪛 actionlint (1.7.4)
322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
343-343: Fix MongoDB health check syntax.
There's a syntax error in the until condition that needs to be fixed.
-until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok">/dev/null 2>&1 || [ $timeout -eq 0]; do
+until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok" >/dev/null 2>&1 || [ $timeout -eq 0 ]; do360-365: Fix Redis health check syntax and indentation.
There are syntax errors in the Redis health check implementation.
-if[ $timeout -eq 0 ]; then
+if [ $timeout -eq 0 ]; then384-393: LGTM! Cleanup implementation follows best practices.
The cleanup function correctly preserves the exit code and uses trap for proper cleanup on script exit.
|
There isn’t an issue assigned to you for this PR. Please follow the guidelines in our PR_GUIDELINES.md file. We have the procedures in place so that everyone has a fair chance of contributing. I will be closing this pull request. Please follow the procedures and resubmit when ready. |
|
@Cioppolo14 I accidently tagged the wrong issue, but I am assigned to this issue: #2638..please re-open this |
|
Please make sure all tests pass and coderabbit.ai approves the PR |
yes sir, will get this ready 5-6 days |
|
Hi @prayanshchh . Have you found any solution for |
is that so, then ig this should be a seperate issue then, can u give ur PR link |
|
Sure. I have opened an issue regarding this since a few others also reported the same in slack. |
|
If you need any assistance feel free to reach out to me @prayanshchh @bandhan-majumder . |
|
Please fix:
Make sure CodeRabbit.ai approves your changes |
|
There is only one conflicting file. Please make your modifications and keep this PR open. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2759 +/- ##
========================================
Coverage 97.74% 97.74%
========================================
Files 358 358
Lines 18103 18103
Branches 2596 2596
========================================
Hits 17695 17695
Misses 404 404
Partials 4 4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/pull-request.yml (3)
321-325: Fix trailing spaces in the run command.Remove trailing spaces after the pipe operator.
- run: | + run: |
389-398: Fix indentation in cleanup function.The cleanup function implementation is good but needs formatting fixes:
- } + }
399-414: Fix environment variables indentation.The environment variables section has inconsistent indentation:
- HEALTH_CHECK_URL: http://localhost:4000 + HEALTH_CHECK_URL: http://localhost:4000 - +🧰 Tools
🪛 yamllint (1.35.1)
[warning] 400-400: wrong indentation: expected 6 but found 8
(indentation)
[error] 414-414: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
327-327: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 372-372: trailing spaces
(trailing-spaces)
[warning] 400-400: wrong indentation: expected 6 but found 8
(indentation)
[error] 414-414: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
299-320: LGTM! Job setup and caching configuration looks good.
The job configuration follows best practices with proper dependency management and caching strategy.
326-343: LGTM! Robust Docker container management implementation.
The implementation includes:
- Proper cleanup of existing containers
- File existence verification
- Timeout for container startup
- Error logging
🧰 Tools
🪛 actionlint (1.7.4)
327-327: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
344-388:
Fix syntax and indentation issues in health checks.
There are several syntax and indentation issues in the health checks:
- if[ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; then
- # Wait for TALAWA API to be healthy
+ # Wait for TALAWA API to be healthy
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
+ until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; doLikely invalid or redundant comment.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 372-372: trailing spaces
(trailing-spaces)
399-414: Verify Docker configuration alignment.
Let's verify that the Docker configuration matches the environment variables:
✅ Verification successful
Docker configuration is properly aligned with environment variables
The verification confirms that the Docker configuration aligns correctly with the environment variables in the GitHub workflow:
- MongoDB configuration matches (
mongodb:27017) - Redis service name and port are correctly configured (
redis-stack-server:6379) - API port 4000 is properly exposed in both Dockerfile.dev and docker-compose.prod.yaml
- All required services (mongodb, redis-stack-server) are present with correct networking
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Docker configuration matches environment variables
# Check if docker-compose file exists and contains the required services
if [ -f "docker-compose.dev.yaml" ]; then
echo "Checking service configuration..."
# Verify MongoDB configuration
if ! grep -q "mongodb:27017" docker-compose.dev.yaml; then
echo "Warning: MongoDB port configuration mismatch"
fi
# Verify Redis configuration
if ! grep -q "redis-stack-server" docker-compose.dev.yaml; then
echo "Warning: Redis service name mismatch"
fi
# Verify API port configuration
if ! grep -q "4000" docker-compose.dev.yaml; then
echo "Warning: API port configuration missing"
fi
fi
Length of output: 342
Script:
#!/bin/bash
# Let's examine the docker-compose.dev.yaml content to better understand the configuration
cat docker-compose.dev.yaml
# Also check if there's a regular docker-compose.yaml
if [ -f "docker-compose.yaml" ]; then
echo -e "\n=== docker-compose.yaml content ===\n"
cat docker-compose.yaml
fi
# Let's also check for any other docker-compose files
fd docker-compose
Length of output: 1564
Script:
#!/bin/bash
# Let's check the production docker compose file as well
cat docker-compose.prod.yaml
# And let's verify if there are any port mappings in the Dockerfile.dev
cat Dockerfile.dev
Length of output: 1387
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 400-400: wrong indentation: expected 6 but found 8
(indentation)
[error] 414-414: trailing spaces
(trailing-spaces)
|
Please fix the failing test |
yes sir |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/pull-request.yml (2)
311-320: Enhance cache key strategyConsider adding
package-lock.jsonversion to the cache key for more precise caching:- key: ${{ runner.os }}-docker-check-${{ hashFiles('**/package-lock.json') }} + key: ${{ runner.os }}-docker-check-v1-${{ hashFiles('**/package-lock.json') }} restore-keys: | - ${{ runner.os }}-docker-check- + ${{ runner.os }}-docker-check-v1-
405-420: Fix trailing spaces in environment variables sectionRemove trailing spaces to maintain consistent formatting:
- HEALTH_CHECK_URL: http://localhost:4000 + HEALTH_CHECK_URL: http://localhost:4000 - MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}🧰 Tools
🪛 yamllint (1.35.1)
[error] 420-420: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
327-327: shellcheck reported issue in this script: SC1089:error:34:1: Parsing stopped here. Is this keyword correctly matched up?
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
395-404: LGTM! Well-implemented cleanup function
The cleanup function correctly preserves the exit code and uses trap for proper cleanup.
378-391:
Fix API health check indentation
The inconsistent indentation could cause YAML parsing errors:
- # Wait for TALAWA API to be healthy
+ # Wait for TALAWA API to be healthy
timeout=60
until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
echo "Waiting for API to start... ($timeout seconds remaining)"
sleep 1
((timeout--))
- done
+ done
- if [ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; thenLikely invalid or redundant comment.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
348-363:
Fix MongoDB health check syntax
There are several syntax issues in this section:
- until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do
+ until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok" >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
sleep 1
- ((timeout--))
+ ((timeout--))
done
- else
- echo "MongoDB is ready!"
- fi
+ echo "MongoDB is ready!"Likely invalid or redundant comment.
366-376:
Fix Redis health check syntax
There are several syntax issues in this section:
- if[ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; then
echo "Error: Redis failed to start within timeout"
docker-compose -f docker-compose.dev.yaml logs redis-stack-server
docker-compose -f docker-compose.dev.yaml down -v
exit 1
- fi
+ fiLikely invalid or redundant comment.
| # Verify docker-compose file exists | ||
| if [ ! -f "docker-compose.dev.yaml" ]; then | ||
| echo "Error: docker-compose.dev.yaml not found" | ||
| docker-compose -f docker-compose.dev.yaml logs | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Start containers | ||
| if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then | ||
| echo "Failed to start containers" | ||
| docker-compose -f docker-compose.dev.yaml logs | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for docker-compose file check
The current implementation logs docker-compose logs even when the file doesn't exist, which would fail.
if [ ! -f "docker-compose.dev.yaml" ]; then
echo "Error: docker-compose.dev.yaml not found"
- docker-compose -f docker-compose.dev.yaml logs
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Verify docker-compose file exists | |
| if [ ! -f "docker-compose.dev.yaml" ]; then | |
| echo "Error: docker-compose.dev.yaml not found" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| exit 1 | |
| fi | |
| # Start containers | |
| if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then | |
| echo "Failed to start containers" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| exit 1 | |
| fi | |
| # Verify docker-compose file exists | |
| if [ ! -f "docker-compose.dev.yaml" ]; then | |
| echo "Error: docker-compose.dev.yaml not found" | |
| exit 1 | |
| fi | |
| # Start containers | |
| if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then | |
| echo "Failed to start containers" | |
| docker-compose -f docker-compose.dev.yaml logs | |
| exit 1 | |
| fi |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/pull-request.yml (1)
331-336:⚠️ Potential issueFix docker-compose logs command placement
The docker-compose logs command will fail if the file doesn't exist.
Apply this fix:
if [ ! -f "docker-compose.dev.yaml" ]; then echo "Error: docker-compose.dev.yaml not found" - docker-compose -f docker-compose.dev.yaml logs exit 1 fi
🧹 Nitpick comments (3)
.github/workflows/pull-request.yml (3)
348-352: Fix indentation in MongoDB health check loopThe indentation is inconsistent in the MongoDB health check loop.
Apply this fix:
until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do -echo "Waiting for MongoDB to be ready..." -sleep 1 - ((timeout--)) + echo "Waiting for MongoDB to be ready..." + sleep 1 + ((timeout--)) done
380-384: Fix indentation in API health check loopThe indentation is inconsistent in the API health check loop.
Apply this fix:
until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do - echo "Waiting for API to start... ($timeout seconds remaining)" - sleep 1 - ((timeout--)) - done + echo "Waiting for API to start... ($timeout seconds remaining)" + sleep 1 + ((timeout--)) +done
299-420: Consider adding timeout for the entire jobWhile individual operations have timeouts, consider adding a timeout for the entire job to prevent long-running workflows.
Add the following at the job level:
Docker-Check: needs: Test-Application runs-on: ubuntu-latest + timeout-minutes: 15 steps:🧰 Tools
🪛 actionlint (1.7.4)
327-327: shellcheck reported issue in this script: SC1069:error:44:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
327-327: shellcheck reported issue in this script: SC1069:error:44:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
299-320: LGTM! Job configuration looks good.
The job configuration follows GitHub Actions best practices with proper dependency setup and caching.
396-404: LGTM! Cleanup function implementation is correct.
The cleanup function correctly preserves the exit code and ensures proper resource cleanup.
405-419: LGTM! Environment variables are well configured.
The environment variables are properly set with secure token generation using GitHub-provided secrets.
371-376:
Fix syntax in Redis health check condition
There's a syntax error in the Redis health check condition.
Apply this fix:
-if[ $timeout -eq 0 ]; then
+if [ $timeout -eq 0 ]; then
echo "Error: Redis failed to start within timeout"
docker-compose -f docker-compose.dev.yaml logs redis-stack-server
docker-compose -f docker-compose.dev.yaml down -v
exit 1
- fi
+fiLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 420-420: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)
299-320: LGTM: Job configuration looks good!
The job configuration follows GitHub Actions best practices with proper dependency setup and caching strategy.
331-336: Remove unnecessary docker-compose logs command
The docker-compose logs command will fail since the file doesn't exist yet.
if [ ! -f "docker-compose.dev.yaml" ]; then
echo "Error: docker-compose.dev.yaml not found"
- docker-compose -f docker-compose.dev.yaml logs
exit 1
fi396-404: LGTM: Cleanup function implementation
The cleanup function correctly preserves the exit code and ensures proper resource cleanup.
405-419: LGTM: Environment variables configuration
The environment variables are well-configured with secure token generation using GitHub-specific variables.
380-391:
Fix API health check indentation
The API health check has inconsistent indentation.
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
- echo "Waiting for API to start... ($timeout seconds remaining)"
- sleep 1
- ((timeout--))
- done
-
- if [ $timeout -eq 0 ]; then
- echo "Error: API failed to start within timeout"
- docker-compose -f docker-compose.dev.yaml logs
- docker-compose -f docker-compose.dev.yaml down -v
- exit 1
- fi
+ until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
+ echo "Waiting for API to start... ($timeout seconds remaining)"
+ sleep 1
+ ((timeout--))
+ done
+
+ if [ $timeout -eq 0 ]; then
+ echo "Error: API failed to start within timeout"
+ docker-compose -f docker-compose.dev.yaml logs
+ docker-compose -f docker-compose.dev.yaml down -v
+ exit 1
+ fiLikely invalid or redundant comment.
| until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do | ||
| echo "Waiting for MongoDB to be ready..." | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done |
There was a problem hiding this comment.
Fix MongoDB health check indentation
The indentation in the MongoDB health check loop is inconsistent.
until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do
-echo "Waiting for MongoDB to be ready..."
-sleep 1
- ((timeout--))
+ echo "Waiting for MongoDB to be ready..."
+ sleep 1
+ ((timeout--))
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| echo "Waiting for MongoDB to be ready..." | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| echo "Waiting for MongoDB to be ready..." | |
| sleep 1 | |
| ((timeout--)) | |
| done |
.github/workflows/pull-request.yml
Outdated
| until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done | ||
| if [ $timeout -eq 0 ]; then | ||
| echo "Error: Redis failed to start within timeout" | ||
| docker-compose -f docker-compose.dev.yaml logs redis-stack-server | ||
| docker-compose -f docker-compose.dev.yaml down -v | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fix Redis health check indentation and spacing
The Redis health check has inconsistent indentation and spacing issues.
- timeout=30
- until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
- sleep 1
- ((timeout--))
- done
- if [ $timeout -eq 0 ]; then
- echo "Error: Redis failed to start within timeout"
- docker-compose -f docker-compose.dev.yaml logs redis-stack-server
- docker-compose -f docker-compose.dev.yaml down -v
- exit 1
- fi
+ timeout=30
+ until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
+ sleep 1
+ ((timeout--))
+ done
+ if [ $timeout -eq 0 ]; then
+ echo "Error: Redis failed to start within timeout"
+ docker-compose -f docker-compose.dev.yaml logs redis-stack-server
+ docker-compose -f docker-compose.dev.yaml down -v
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: Redis failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs redis-stack-server | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi | |
| timeout=30 | |
| until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do | |
| sleep 1 | |
| ((timeout--)) | |
| done | |
| if [ $timeout -eq 0 ]; then | |
| echo "Error: Redis failed to start within timeout" | |
| docker-compose -f docker-compose.dev.yaml logs redis-stack-server | |
| docker-compose -f docker-compose.dev.yaml down -v | |
| exit 1 | |
| fi |
|
the docker workflow issue in this pr is related to this issue #2598 , this is the reason I wanted to fix the issue, as the issue is not assigned to anyone, would like to work on it, this won't be completed until and unless that issue is fixed. thoughts? @palisadoes |
Thanks. I've assigned the issue to you. Please fix both in this PR. |
yes sir, on it |
|
sir I have fixed redis-stack-server issue and PWD variable issue, I was going through logs and I noticed that we don't have any configuration of replset for mongodb when app is started using docker, hence the application gives warning |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
setup.ts (3)
1018-1025: Consider validating the 'pwdVariable'Currently, there's no check to ensure the user-supplied path is valid for Docker. If a user provides an invalid path, it may lead to unexpected behavior when spinning up containers. Validating or clarifying that the path should exist can reduce such issues.
1040-1040: Potential confusion over shell variable 'PWD'Manually setting process.env.PWD can be confusing since shells typically set this variable automatically. This is not necessarily wrong, but be mindful that it may differ from the shell’s natural behavior.
1047-1047: Logging environment variable 'PWD'PWD is not sensitive, but ensure that printing environment variables to the console is intentional. In some environments, environment variables may contain secrets or credentials (not the case here, but worth noting).
docker-compose.dev.yaml (1)
62-62: Add a newline at the end of fileYAML linting tools recommend a newline at the end of the file. Ensure there's a trailing newline to satisfy lint checks.
talawa-network: driver: bridge +🧰 Tools
🪛 yamllint (1.35.1)
[error] 62-62: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/pull-request.yml (1)
378-378: Remove trailing spaces and whitespaceThe lines in this file contain trailing spaces, which may fail certain lint checks. It's a minor issue but can easily be resolved:
- echo "Some line" + echo "Some line"Also applies to: 383-383, 425-425
🧰 Tools
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.sample(1 hunks).github/workflows/pull-request.yml(1 hunks)docker-compose.dev.yaml(2 hunks)setup.ts(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 383-383: trailing spaces
(trailing-spaces)
[error] 425-425: trailing spaces
(trailing-spaces)
docker-compose.dev.yaml
[error] 62-62: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
setup.ts (1)
1033-1033: Ensure consistency when assigning 'PWD' to Env Config
By setting config.PWD here, the value is saved in .env for future runs. Ensure any references to the environment variable PWD consistently rely on config.PWD or process.env.PWD.
docker-compose.dev.yaml (3)
38-40: Service name and port mapping
Using an explicit container_name can be useful for local debugging. However, be aware that it can cause naming conflicts. The addition of port mapping 4000:4000 is consistent with the default server port configured in the environment.
49-49: Updated MongoDB URL
The URL now explicitly targets the 'talawa-api' database. This is a correct approach to keep data organized.
52-53: Network usage
These lines define the talawa-network usage for the API service. This is clean and consistent with the other services.
.env.sample (1)
107-110: Additional environment variable 'PWD'
The new environment variable defaults to ".". This is sensible for a Docker working directory, but you might indicate in comments that it's primarily used by Docker-based workflows.
.github/workflows/pull-request.yml (1)
299-410: Docker-Check Job
Introducing a dedicated Docker-Check job is beneficial for ensuring the API starts successfully in a containerized environment. The outlined steps and timeouts for bringing up the container, along with health checks, are thorough.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 383-383: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.github/workflows/pull-request.yml (6)
311-320: Improve cache key to include Node.js version.Consider including the Node.js version in the cache key to ensure proper cache isolation between different Node.js versions.
- key: ${{ runner.os }}-docker-check-${{ hashFiles('**/package-lock.json') }} + key: ${{ runner.os }}-node-${{ matrix.node-version }}-docker-check-${{ hashFiles('**/package-lock.json') }}
348-352: Fix MongoDB health check indentation.The indentation in the MongoDB health check loop is inconsistent.
until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongosh --eval "db.runCommand({ ping: 1 }).ok">/dev/null 2>&1 || [ $timeout -eq 0 ]; do -echo "Waiting for MongoDB to be ready..." -sleep 1 - ((timeout--)) + echo "Waiting for MongoDB to be ready..." + sleep 1 + ((timeout--)) done
366-381: Fix Redis health check indentation and spacing.The Redis health check section has inconsistent indentation and extra spaces.
- timeout=30 - until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do - sleep 1 - ((timeout--)) - done - if [ $timeout -eq 0 ]; then - echo "Error: Redis failed to start within timeout" - echo "Fetching Redis logs..." - docker-compose -f docker-compose.dev.yaml logs redis-stack-server - echo "Shutting down Redis..." - docker-compose -f docker-compose.dev.yaml down -v - exit 1 + timeout=30 + until docker-compose -f docker-compose.dev.yaml exec -T redis-stack-server redis-cli ping >/dev/null 2>&1 || [ $timeout -eq 0 ]; do + sleep 1 + ((timeout--)) + done + if [ $timeout -eq 0 ]; then + echo "Error: Redis failed to start within timeout" + echo "Fetching Redis logs..." + docker-compose -f docker-compose.dev.yaml logs redis-stack-server + echo "Shutting down Redis..." + docker-compose -f docker-compose.dev.yaml down -v + exit 1🧰 Tools
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
383-396: Improve API health check implementation.The API health check has inconsistent indentation and should use the
HEALTH_CHECK_URLenvironment variable.- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/graphql" 2>&1 || [ $timeout -eq 0 ]; do + until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "${HEALTH_CHECK_URL}/graphql" 2>&1 || [ $timeout -eq 0 ]; do echo "Waiting for API to start... ($timeout seconds remaining)" sleep 1 ((timeout--)) - done + done🧰 Tools
🪛 yamllint (1.35.1)
[error] 383-383: trailing spaces
(trailing-spaces)
401-408: Enhance cleanup function with additional error information.Consider adding more detailed error information when cleanup fails.
cleanup() { local exit_code=$? echo "Cleaning up containers..." if ! docker-compose -f docker-compose.dev.yaml down -v; then - echo "Warning: Failed to cleanup containers" + echo "Warning: Failed to cleanup containers. Exit code: $?" + docker-compose -f docker-compose.dev.yaml ps fi exit $exit_code }
425-425: Remove trailing spaces.Remove trailing spaces at the end of line 425.
- MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} - + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}🧰 Tools
🪛 yamllint (1.35.1)
[error] 425-425: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 383-383: trailing spaces
(trailing-spaces)
[error] 425-425: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/pull-request.yml (1)
331-336:
Fix docker-compose file verification logic.
The current implementation attempts to show docker-compose logs even when the file doesn't exist, which would fail.
if [ ! -f "docker-compose.dev.yaml" ]; then
echo "Error: docker-compose.dev.yaml not found"
- docker-compose -f docker-compose.dev.yaml logs
exit 1
fiLikely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/pull-request.yml (2)
401-409: Add error logging to cleanup functionThe cleanup function should log the exit code for better debugging.
Apply this diff:
cleanup() { local exit_code=$? echo "Cleaning up containers..." + echo "Exit code: $exit_code" if ! docker-compose -f docker-compose.dev.yaml down -v; then echo "Warning: Failed to cleanup containers" fi exit $exit_code }
410-425: Consider environment variable improvementsA few suggestions for the environment variables:
- The
HEALTH_CHECK_URLis not used in the script- Consider adding timeout configurations as environment variables
Apply this diff:
env: - HEALTH_CHECK_URL: http://localhost:4000 + MONGODB_TIMEOUT: 30 + REDIS_TIMEOUT: 30 + API_TIMEOUT: 60 COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 425-425: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 383-383: trailing spaces
(trailing-spaces)
[error] 425-425: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/pull-request.yml (4)
299-320: LGTM! Job configuration and dependency setup looks good.
The job configuration correctly:
- Depends on Test-Application job
- Uses ubuntu-latest runner
- Sets up Node.js and caches dependencies
331-336: Remove unnecessary docker-compose logs command
The docker-compose logs command will fail since the file doesn't exist yet.
348-352: Fix MongoDB health check indentation
The indentation in the MongoDB health check loop is inconsistent.
299-425: Verify Docker network configuration
The workflow should verify that the Docker network is properly configured and that containers can communicate with each other.
✅ Verification successful
Docker network configuration is properly defined
The docker-compose.dev.yaml file has a well-configured network setup:
- All services (mongodb, redis-stack-server, minio, talawa-api-dev) are connected to the same
talawa-network - The network is properly defined using the bridge driver
- Service dependencies are correctly specified
- Internal service communication is enabled through the shared network
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Docker network configuration
# Check if docker-compose.dev.yaml defines the correct network settings
if [ -f "docker-compose.dev.yaml" ]; then
echo "Checking network configuration..."
# Verify network definitions
yq e '.networks' docker-compose.dev.yaml
# Verify service network attachments
yq e '.services.*.networks' docker-compose.dev.yaml
fi
Length of output: 395
Script:
#!/bin/bash
# Let's try to examine the docker-compose.dev.yaml file directly
if [ -f "docker-compose.dev.yaml" ]; then
echo "Content of docker-compose.dev.yaml:"
cat docker-compose.dev.yaml
fi
Length of output: 1364
🧰 Tools
🪛 yamllint (1.35.1)
[error] 378-378: trailing spaces
(trailing-spaces)
[error] 383-383: trailing spaces
(trailing-spaces)
[error] 425-425: trailing spaces
(trailing-spaces)
| until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -v -X OPTIONS "http://talawa-api-dev:4000/graphql" 2>&1 || [ $timeout -eq 0 ]; do | ||
| echo "Waiting for API to start... ($timeout seconds remaining)" | ||
| sleep 1 | ||
| ((timeout--)) | ||
| done |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve API health check reliability
The current API health check has a few issues:
- Using OPTIONS request might not accurately verify the GraphQL endpoint
- Internal container DNS resolution might be unreliable
Apply this diff to improve the health check:
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -v -X OPTIONS "http://talawa-api-dev:4000/graphql" 2>&1 || [ $timeout -eq 0 ]; do
+ until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf -X POST "http://localhost:4000/graphql" \
+ -H 'Content-Type: application/json' \
+ -d '{"query":"{ __typename }"}' 2>&1 || [ $timeout -eq 0 ]; doCommittable suggestion skipped: line range outside the PR's diff.
| @@ -138,8 +138,6 @@ export const createVolunteerAndActions = async (): Promise< | |||
| }); | |||
|
|
|||
| const today = new Date(); | |||
| const yesterday = new Date(today); | |||
There was a problem hiding this comment.
Please exclude this file. It is not relevant to the original issue. You have also changed its logic
| const { pwdVariable } = await inquirer.prompt({ | ||
| type: "input", | ||
| name: "pwdVariable", | ||
| message: |
There was a problem hiding this comment.
Why was this added? It should have been fixed in a previously merged PR. Please merge your code with the latest upstream/postgres-develop to be certain.
|
What kind of change does this PR introduce?
Changing workflow to ensure that talawa api starts in docker
Issue Number:
#2638
Fixes #
Did you add tests for your changes?
No
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Docker-Checkjob in the GitHub Actions workflow to enhance Docker setup and verification.PWDadded to the sample environment configuration.Bug Fixes
Docker-Checkjob configuration.Tests
Changes
docker-compose.dev.yamlto modify service configurations, including port mappings and database URL structure.caddyservice from the Docker configuration.